Fix MixedType->equals(ErrorType) #3934
Conversation
|
|
|
This pull request has been marked as ready for review. |
|
👍 I suppose this improves the weird cases I had before too, where error is accepted as a mixed or at least something like that 😊 |
src/Analyser/MutatingScope.php
Outdated
| @@ -4489,7 +4489,7 @@ public function addTypeToExpression(Expr $expr, Type $type): self | |||
|
|
|||
| if ($originalExprType->equals($nativeType)) { | |||
| $newType = TypeCombinator::intersect($type, $originalExprType); | |||
| if ($newType->isConstantScalarValue()->yes() && $newType->equals($originalExprType)) { | |||
| if ($newType->isObject()->no() && $newType->equals($originalExprType)) { | |||
There was a problem hiding this comment.
Will look into whether we can get rid of the isObject() check here today
There was a problem hiding this comment.
ok, I looked into it. I think we cannot drop the $newType->isObject()->no() check here, because Objects will be considered equal by ObjectType->equals() even though they have slight differences which are important when narrowing, e.g.
new Foo() in local-scope vs. Foo typed variable (implicit Final)
There was a problem hiding this comment.
most conservative move would be to leave it $newType->isConstantScalarValue()->yes() until we get a slow case which benefits from the change and only fix MixedType->equals(ErrorType) for now
clxmstaab
force-pushed
the
equals
branch
2 times, most recently
from
868ed2e to
65df00a
Compare
|
just reduced it to the actual bugfix. |
staabm
force-pushed
the
equals
branch
2 times, most recently
from
9e0fca0 to
81df7ec
Compare
|
Thank you. |
refs #3933 (comment)